Remove potentially problematic memory optimization in BlockSubLevelLiftProvider - #1419
Remove potentially problematic memory optimization in BlockSubLevelLiftProvider#1419KosmX wants to merge 1 commit into
Conversation
Avoid using "globals to avoid allocation", this way JIT can do proper escape analysis, do proper optimizations (and make the function thread-safe too)
|
have you done benchmarking to support this for this case, or is it simply speculative? |
|
I haven't had great experiences with this sortof thing but I'm happy to be proven wrong. I typically see the vector allocation showing up heavily in profiling for CPU time, it's what sparked this pattern in the first place |
|
It is speculative, I don't know when can I do benchmarking. or with what profiler. What is your profiler setup? maybe I can use that. |
|
Tomorrow I'll attempt to profile this with async-profiler, but I have no idea if this change is measurable. |
|
|
Avoid using "globals to avoid allocation", this way JIT can do proper escape analysis, do proper optimizations (and make the function thread-safe too)
More details on escape analysis:
https://docs.oracle.com/javase/8/docs/technotes/guides/vm/performance-enhancements-7.html#escapeAnalysis
There are many reasons why I would not create globals to avoid allocation:
I was thinking on what might be worse with this, but I couldn't find many reasons. Some allocations will occur before JIT compiles the function, but that probably doesn't matter (since compilation will happen pretty fast)
Also because this patch removes a few fields, it might break some mods, so merging this might be a bit risky.
And since the function isn't a performance bottleneck anyway, I would say this PR is pretty low priority.